Skip to content

[FLINK-40089][table] Implement JSON_LENGTH function#28688

Open
VasShabu wants to merge 2 commits into
apache:masterfrom
VasShabu:jsonLengthImplementation
Open

[FLINK-40089][table] Implement JSON_LENGTH function#28688
VasShabu wants to merge 2 commits into
apache:masterfrom
VasShabu:jsonLengthImplementation

Conversation

@VasShabu

@VasShabu VasShabu commented Jul 8, 2026

Copy link
Copy Markdown

What is the purpose of the change

This pull request adds support for the built-in SQL function JSON_LENGTH, which returns the number of top-level elements in a JSON array or object. Scalars have a length of 1, and NULL input returns NULL. Nested arrays or objects are not counted recursively.

Brief change log

  • Added JsonLengthFunction implementing the JSON_LENGTH built-in function
  • Registered JSON_LENGTH as a built-in function in the function catalog
  • Added test cases for JSON_LENGTH in JsonFunctionsITCase

Verifying this change

Please make sure both new and modified tests in this PR follow the conventions for tests defined in our code quality guide.

This change added tests and can be verified as follows:

  • mvn test -pl flink-table/flink-table-runtime -Dtest=JsonFunctionsITCase
  • Added test cases for JSON_LENGTH covering scalars, arrays, objects, and NULL input in JsonFunctionsITCase

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): yes
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? docs (sql_functions.yml)

Was generative AI tooling used to co-author this PR?
  • Yes (Claude)

@flinkbot

flinkbot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@raminqaf raminqaf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @VasShabu! Left some comments!

Comment thread flink-python/docs/reference/pyflink.table/expressions.rst
Comment thread docs/data/sql_functions.yml Outdated
Comment thread docs/data/sql_functions.yml
Comment thread flink-python/pyflink/table/expression.py Outdated
Comment on lines +377 to +396
public static Integer jsonLength(String input, String pathSpec) {
return jsonLength(jsonApiCommonSyntax(input, pathSpec));
}

private static Integer jsonLength(JsonPathContext context) {
if (context.hasException()) {
return null;
}
final Object value = context.obj;
if (value == null) {
return null;
}
if (value instanceof Map) {
return ((Map<?, ?>) value).size();
}
if (value instanceof Collection) {
return ((Collection<?>) value).size();
}
return 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to we need this? Don't we have already the runtime class JsonLengthFunction?

super(BuiltInFunctionDefinitions.JSON_LENGTH, context);
}

public @Nullable Integer eval(@Nullable StringData jsonInput) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: here and everywhere else make parameters/variables that are immutable final (not enforced on Flink but nicer to read).

Suggested change
public @Nullable Integer eval(@Nullable StringData jsonInput) {
public @Nullable Integer eval(final @Nullable StringData jsonInput) {

Comment thread docs/data/sql_functions.yml
Comment thread docs/data/sql_functions.yml
}
}

public static Integer jsonLength(String input, String pathSpec) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have these utilities here and some other in JsonLengthFuntion.java itself. Check if we don't already have utilities for performing the length check. If not, check if it makes sense to move the new private utilities you added to this file

@gustavodemorais gustavodemorais left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @VasShabu! Good job on the first version of the PR. We'll go through some iterations to make sure things look good

Apart from what we already flagged above: we have two implementations of the same function. eval(json) uses a new Jackson readTree, eval(json, path) uses the existing Calcite path in SqlJsonUtils. Different parsers + different data models = they can disagree on the same input. We can make the design more consistent by using only one path. Duplicate ObjectMapper - reuse the configured one in SqlJsonUtils instead of a second bare instance. This is important because built-in functions are on the hot path. The logic might be invoked millions of times in a couple of seconds and we have to make sure they allocate as little as possible and their performance are optimized or else the engine will be wasting resources.

That said, In this case, we could go two paths

  1. Best performance: We build a custom parser to optimize it for length checking by skipping the children. MAPPER.getFactory().createParser(str), read first token; if START_ARRAY/START_OBJECT walk top level counting, skipping children via parser.skipChildren(). No tree alloc. Best for length-only.
  2. Simplest reuse the shared SqlJsonUtils path so both overloads share one parser.

Take a look at both paths and let me know what you think

Comment thread docs/data/sql_functions.yml Outdated
} else if (jsonNode.isTextual()
|| jsonNode.isNumber()
|| jsonNode.isBoolean()
|| jsonNode.isBinary()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this check dead code? Can a jsonNode be binary? You can try to write a test to check if it's dead code or it's possible

@snuyanzin snuyanzin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure the selected approach is the right one?
With this approach the same JSON will be parsed several times for queries like

SELECT JSON_LENGTH(my_json), JSON_LENGTH(my_json), JSON_LENGTH(my_json) FROM...

or

SELECT JSON_LENGTH(my_json), JSON_VALUE(my_json, '$.a') FROM...

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jul 8, 2026
Comment on lines +1689 to +1692
table: jsonLength(jsonObject[, path])
description: |
Returns the number of elements in a JSON document, or the length of the value at the specified path if one is provided.
Returns NULL if the argument is NULL or the path does not locate a value.

@snuyanzin snuyanzin Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not look true
simple tests show different behavior

SELECT json_length('$');
SELECT json_length('{');

each returns -1 and the doc says nothing about this or did I miss anything?

@snuyanzin snuyanzin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More findings:

  1. Doc doesn't reflect the real behavior: it returns -1 in case of invalid json, while docs says about NULL, e.g. query SELECT JSON_LENGTH('null');
  2. Strange handling of null values
    like a query
    SELECT json_length('{"a":[true, false, null]}', '$.a[1]')
    both in MySQL and in Flink it returns 1
    now
    SELECT json_length('{"a":[true, false, null]}', '$.a[2]')
    in MySQL it returns 1, in Flink it returns -1 why?
  3. Seems need to check what the behavior should be in case of lax/strict since right now it is weird
    SELECT json_length('{"x": {"a":1}, "y": {"a":[2, 3]}}', 'lax $.*.a');  -- returns 2
    SELECT json_length('{"x": {"a":[1, 2, 3, 4]}, "y": {"a":[2, 3]}}', 'lax $.*.a');  -- returns 2
    SELECT json_length('{"x": {"a":[3, 2 , 3]}, "y": {"a":null}, "z":{"a":1}}', 'lax *.a'); -- returns 3
    SELECT json_length('{"x": {"a":[3, 2 , 3]}, "y": {"a":null}, "z":{"a":1}}', 'lax *.a[0]'); --returns 1
    SELECT json_length('{"x": {"a":[3, 2 , 3]}, "y": {"a":null}, "z":{"a":1}}', 'lax *.a[1]'); --returns 1
    SELECT json_length('{"x": {"a":[3, 2 , 3]}, "y": {"a":null}, "z":{"a":1}}', 'lax *.a[2]'); --returns 1
    SELECT json_length('{"x": {"a":[3, 2 , 3]}, "y": {"a":null}, "z":{"a":1}}', 'lax *.a[3]');  --returns 0

"""
return _unary_op("jsonUnquote")(self)

def json_length(self, path = None) -> 'Expression':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that many databases throw an error, I am curious why we are not throwing an error. Also I see that there are best practises that we should document like using IF(string_column IS JSON, to protect against non json and avoid potential errors.

final JsonPathContext context = jsonApiCommonSyntax(parsedInput, pathSpec);
final Object value = context.hasException() ? null : context.obj;

if (value instanceof LinkedList) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason we expect LinkedList instead of just List?

* same input expression the parsed JSON context is reused via a shared member variable, so a given
* JSON string is parsed at most once per row regardless of how many JSON functions read it.
*/
class JsonLengthCallGen extends CallGenerator {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we should not have new classes in scala

Comment on lines +30 to +32
logger.codelog.name = org.apache.flink.table.runtime.generated.CompileUtils
logger.codelog.level = DEBUG
logger.codelog.appenderRef.test.ref = TestLogger

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be helpful for local debug, however why do we need in repo and making it executable for every ci run?

Comment on lines +106 to +116
jsonValue, // f0: existing resource JSON
"{\"a\":1,\"b\":2}", // f1: object
"[1,2,3]", // f2: array
"\"abc\"", // f3: scalar string
"null", // f4: JSON null
"{", // f5: invalid JSON
((String) null), // f6: SQL NULL
"$",
"{\"a\":[true, false, null]}", // f8
"{}", // f9: empty object
"[]") // f10: empty array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we need all these comments, what a value it could give to the reader who sees that {} is an empty object and then a comment telling same?

assertThat(rows).containsExactlyInAnyOrder(Row.of(4, "account"), Row.of(4, "admin"));
assertThat(countJsonParse(extractGeneratedCode(sql)))
.as("JSON_LENGTH + JSON_VALUE on the same input should parse once")
.isEqualTo(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.isEqualTo(1);
.isOne();

nit

@raminqaf raminqaf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another pass

Comment thread docs/data/sql_functions.yml
Comment thread docs/data/sql_functions_zh.yml
Comment thread flink-python/pyflink/table/expression.py
return null;
}
final JsonPathContext context = jsonApiCommonSyntax(parsedInput, pathSpec);
final Object value = context.hasException() ? null : context.obj;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if context.hasException() is true, then value would be null. We should probably then short circuit and return null instead of continuing and risking a NPE

Comment on lines +406 to +408
if (context.hasException() || value == null) {
return pathExists(parsedInput.obj, pathSpec) ? 1 : null;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return jsonLengthValue(value);
}

private static Integer jsonLengthValue(final Object value) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we never return null so we can use primitiv int

Suggested change
private static Integer jsonLengthValue(final Object value) {
private static int jsonLengthValue(final Object value) {

* whether the matched value is a JSON null literal. Uses {@link Option#AS_PATH_LIST} so the
* result is the list of matched canonical paths, which is empty iff the path does not exist.
*/
private static boolean pathExists(Object json, String pathSpec) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Since you are at it

Suggested change
private static boolean pathExists(Object json, String pathSpec) {
private static boolean pathExists(final Object json, final String pathSpec) {

Comment on lines +398 to +403
if (matched.size() != 1) {
if (context.mode == PathMode.STRICT || matched.isEmpty()) {
return null;
}
return matched.size();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please elaborate on this? In which conditions we get a null and in which conditions we get the size of the matched?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants